Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Batch icon in dark mode #4516

Merged
merged 1 commit into from
Nov 14, 2024
Merged

fix: Batch icon in dark mode #4516

merged 1 commit into from
Nov 14, 2024

Conversation

usame-algan
Copy link
Member

What it solves

Resolves #4384

How this PR fixes it

  • Uses SvgIcon to insert batch icon in transactions list

How to test it

  1. Open a safe with batched transactions
  2. Go to the history
  3. Observe the batch icon is visible in dark and light mode

Screenshots

Screenshot 2024-11-11 at 21 16 48 Screenshot 2024-11-11 at 21 16 57

Checklist

  • I've tested the branch on mobile 📱
  • I've documented how it affects the analytics (if at all) 📊
  • I've written a unit/e2e test for it (if applicable) 🧑‍💻

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 11, 2024 •
edited
Loading

Branch preview

✅ Deploy successful!

Website:
https://batch_icon_dark_mode--walletweb.review.5afe.dev/home?safe=eth:0xA77DE01e157f9f57C7c4A326eeE9C4874D0598b6

Storybook:
https://batch_icon_dark_mode--walletweb.review.5afe.dev/storybook/

@github-actions GitHub Actions
Copy link

📦 Next.js Bundle Analysis for safe-wallet-web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

⚠️ Global Bundle Size Increased

Page Size (compressed)
global 1014.34 KB (🟡 +270 B)
Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

@github-actions GitHub Actions
Copy link

Coverage report

St.❔
 Category Percentage Covered / Total
🟡 Statements
74.25% (+0% 🔼)
 14063/18940
🔴 Branches 52.57% 3476/6612
🔴 Functions 58.04% 2053/3537
🟡 Lines
75.88% (+0% 🔼)
 12775/16836

Test suite run success

1619 tests passing in 217 suites.

Report generated by 🧪jest coverage report action from 43db540

compojoom
compojoom approved these changes Nov 11, 2024
View reviewed changes
Copy link
Collaborator

@compojoom compojoom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@francovenica
Copy link
Contributor

LGTM
image

@usame-algan usame-algan merged commit b9fc13e into dev Nov 14, 2024
14 of 15 checks passed
@usame-algan usame-algan deleted the batch-icon-dark-mode branch November 14, 2024 10:14
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2024
@mike10ca
Copy link
Contributor

mike10ca commented Nov 15, 2024 •
edited
Loading

Team, we have a smoke test failed. This is directly related to the change in this PR. I fixed it yesterday evening though already. Please invest some time to review the tests before merging. @francovenica @usame-algan @compojoom CC: @liliya-soroka

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@compojoom compojoom compojoom approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Batch icon in the txs list is not seen in dark theme
4 participants
@usame-algan @francovenica @mike10ca @compojoom